-
Notifications
You must be signed in to change notification settings - Fork 15.3k
[libc] Temporarily disable LlvmLibcFileTest.WriteOnly in libc.test.src.__support.File.file_test.__hermetic__ due to precommit bots's consistent failures. #128186
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…c.__support.File.file_test.__hermetic__ due to precommit bots's consistent failures.
|
@llvm/pr-subscribers-libc Author: None (lntue) ChangesFull diff: https://github.com/llvm/llvm-project/pull/128186.diff 1 Files Affected:
diff --git a/libc/test/src/__support/File/file_test.cpp b/libc/test/src/__support/File/file_test.cpp
index b3c9f2ba49bce..9c634ca07a5d0 100644
--- a/libc/test/src/__support/File/file_test.cpp
+++ b/libc/test/src/__support/File/file_test.cpp
@@ -113,43 +113,46 @@ StringFile *new_string_file(char *buffer, size_t buflen, int bufmode,
LIBC_NAMESPACE::File::mode_flags(mode));
}
-TEST(LlvmLibcFileTest, WriteOnly) {
- const char data[] = "hello, file";
- constexpr size_t FILE_BUFFER_SIZE = sizeof(data) * 3 / 2;
- char file_buffer[FILE_BUFFER_SIZE];
- StringFile *f =
- new_string_file(file_buffer, FILE_BUFFER_SIZE, _IOFBF, false, "w");
-
- ASSERT_EQ(sizeof(data), f->write(data, sizeof(data)).value);
- EXPECT_EQ(f->get_pos(), size_t(0)); // Data is buffered in the file stream
- ASSERT_EQ(f->flush(), 0);
- EXPECT_EQ(f->get_pos(), sizeof(data)); // Data should now be available
- EXPECT_STREQ(f->get_str(), data);
-
- f->reset();
- ASSERT_EQ(f->get_pos(), size_t(0));
- ASSERT_EQ(sizeof(data), f->write(data, sizeof(data)).value);
- EXPECT_EQ(f->get_pos(), size_t(0)); // Data is buffered in the file stream
- // The second write should trigger a buffer flush.
- ASSERT_EQ(sizeof(data), f->write(data, sizeof(data)).value);
- EXPECT_GE(f->get_pos(), size_t(0));
- ASSERT_EQ(f->flush(), 0);
- EXPECT_EQ(f->get_pos(), 2 * sizeof(data));
- MemoryView src1("hello, file\0hello, file", sizeof(data) * 2),
- dst1(f->get_str(), sizeof(data) * 2);
- EXPECT_MEM_EQ(src1, dst1);
-
- char read_data[sizeof(data)];
- {
- // This is not a readable file.
- auto result = f->read(read_data, sizeof(data));
- EXPECT_EQ(result.value, size_t(0));
- EXPECT_TRUE(f->error());
- EXPECT_TRUE(result.has_error());
- }
-
- ASSERT_EQ(f->close(), 0);
-}
+// TODO: Investigate the precommit bots' failures of this test and re-enable it.
+// https://github.com/llvm/llvm-project/issues/128185.
+//
+// TEST(LlvmLibcFileTest, WriteOnly) {
+// const char data[] = "hello, file";
+// constexpr size_t FILE_BUFFER_SIZE = sizeof(data) * 3 / 2;
+// char file_buffer[FILE_BUFFER_SIZE];
+// StringFile *f =
+// new_string_file(file_buffer, FILE_BUFFER_SIZE, _IOFBF, false, "w");
+
+// ASSERT_EQ(sizeof(data), f->write(data, sizeof(data)).value);
+// EXPECT_EQ(f->get_pos(), size_t(0)); // Data is buffered in the file stream
+// ASSERT_EQ(f->flush(), 0);
+// EXPECT_EQ(f->get_pos(), sizeof(data)); // Data should now be available
+// EXPECT_STREQ(f->get_str(), data);
+
+// f->reset();
+// ASSERT_EQ(f->get_pos(), size_t(0));
+// ASSERT_EQ(sizeof(data), f->write(data, sizeof(data)).value);
+// EXPECT_EQ(f->get_pos(), size_t(0)); // Data is buffered in the file stream
+// // The second write should trigger a buffer flush.
+// ASSERT_EQ(sizeof(data), f->write(data, sizeof(data)).value);
+// EXPECT_GE(f->get_pos(), size_t(0));
+// ASSERT_EQ(f->flush(), 0);
+// EXPECT_EQ(f->get_pos(), 2 * sizeof(data));
+// MemoryView src1("hello, file\0hello, file", sizeof(data) * 2),
+// dst1(f->get_str(), sizeof(data) * 2);
+// EXPECT_MEM_EQ(src1, dst1);
+
+// char read_data[sizeof(data)];
+// {
+// // This is not a readable file.
+// auto result = f->read(read_data, sizeof(data));
+// EXPECT_EQ(result.value, size_t(0));
+// EXPECT_TRUE(f->error());
+// EXPECT_TRUE(result.has_error());
+// }
+
+// ASSERT_EQ(f->close(), 0);
+// }
TEST(LlvmLibcFileTest, WriteLineBuffered) {
const char data[] = "hello\n file";
|
|
|
||
| ASSERT_EQ(f->close(), 0); | ||
| } | ||
| // TODO: Investigate the precommit bots' failures of this test and re-enable it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use
#if 0
...
#endif
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Google Test supports disabling tests by prefixing their name with DISABLED_. This is better than commenting out the code or using #if 0, as disabled tests are still compiled (and thus won't rot). We should consider implementing the same feature in libc's test framework.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'd probably want that to be target dependent as well, there's a handful of places where I need to disable a test only on NVPTX for example.
|
The root cause and fix for the issue is #128426 |
No description provided.